All committers have signed the CLA.

Looks good to me. Thank you for your contribution!

There is a test whose expected result needs to be updated.

Not sure how to do that.

Actually the test failed because the generated spirv failed validation. We need to investigate which validation rule is being violated.

On my computer that test doesn't fail so I'm not sure how to proceed.

On my computer that test doesn't fail so I'm not sure how to proceed.

You probably didn't enable the SPIRV validation.
We documented the instruction in https://github.com/shader-slang/slang/blob/master/CONTRIBUTING.md#testing
In short, you should set an environment variable as following,

set SLANG_RUN_SPIRV_VALIDATION=1

The test fails due to this line
%sampledImageType = OpTypeSampledImage $$Texture2D<float>;
It still infers unknown for the texture format and that doesn't match the newly inferred type of the texture. The test also fails independently of this pr if half is used instead of float.

I've been unable to figure out where to put the inference code though I'm still looking.

Ah, actually we do want to have Texture2D<float> or Texture2D<float4> to map to unknown format when generating SPIRV, so we shouldn't be adding the image format inference logic here.

The reasoning is that when the user defines something like Texture2D texture; without a format attribute, it automatically expands to Texture2D<float4> texture;, and in this case we shouldn't produce SPIRV code that declares the format to be RGBA32F. Most often the texture will NOT be in that format and instead be something like rgba8unorm. Specifying unknown here is the only correct behavior.

I could infer for float but not float4 if the issue is just the Texture2D to Texture2D<float4> conversion which would fix the issue in wgsl. Would that work?

It would require modifying the test to use float4 instead of float, but since the test doesn't work with half, it is already testing a special case.

Inferring for float maybe OK, since it is a less common case. But still the argument applies for R8UNORM formats.

It is safer to not infer for float, because all unorm/snorm formats can map to float.

The right fix to this problem IMO, is to allow user to write Texture2D<r8unorm> to be more explicit on the format, but t his requires some refactoring on how texture types are represented internally.

People can use attributes to specify format as well. R10G10B10A2_UINT creates ambiguity for UInt4. If we want to prevent ever inferring the wrong format maybe removing format inference all together would be better. I would argue getting rgba32 for float in wgsl due to not inferring is just as bad as getting other undesired formats due to inferring.

For WGSL where there isn't a "unknown" format, I agree that any inference is better.
But we need to be careful to not break SPIRV applications by adding more inferencing.

Perhaps we should do this during wgsl emit time, so we can do a last minute infer for wgsl when the default front-end infer failed.

Perhaps we should do this during wgsl emit time, so we can do a last minute infer for wgsl when the default front-end infer failed.

Looks like that's what glsl does. Converted over to using that.

Looks good, need a formatting pass.

/format

🌈 Formatted, please merge the changes from this PR